-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed (Favicon related) errors which returned by Zimcheck #175
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have tested, but from the principle it looks good. But can you please secure the images behind the link
are good. Here this is always the same stuff used. It should not. If you take the logo of the welcome page, you should be able to recreate good one in all resolution needed easily.
32d657f
to
4af41dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e45dd0b
to
0d2db8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have started the review of this PR, but there is already a first regression: the ZIM logo/favicon is not there anymore, see the question mark:
Actually the zìmcheck does detect it:
$ zimcheck dist/phet_
[INFO] Checking zim file dist/phet_
[INFO] Zimcheck version is 3.1.3
[INFO] Verifying ZIM-archive structure integrity...
error 2 opening file "dist/phet_
kelson@camber:~/code/phet$ zimcheck dist/phet_fr_2022-12.zim
[INFO] Checking zim file dist/phet_fr_2022-12.zim
[INFO] Zimcheck version is 3.1.3
[INFO] Verifying ZIM-archive structure integrity...
[INFO] Avoiding redundant checksum test (already performed by the integrity check).
[INFO] Searching for metadata entries...
[INFO] Searching for Favicon...
[INFO] Searching for main page...
[INFO] Verifying Articles' content...
[INFO] Searching for redundant articles...
Verifying Similar Articles for redundancies...
[INFO] Checking for redirect loops...
[ERROR] Favicon:
Favicon is missing
[INFO] Overall Test Status: Fail
[INFO] Total time taken by zimcheck: 3 seconds.
Please fix and introduce a zimcheck test run on the ZIM created with the test, you can look how this is done already at https://github.com/openzim/mwoffliner/tree/main/test
Favicon checker looks good! But there is a problem with safari-pinned-tab.svg
which is full black.
0d2db8b
to
785c923
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zimcheck does not run in the CI, it does not work like on MWoffliner (given as example in my comment). I have fixed this, but the CI does not pass, because there is still a problem around the ZIM file with the dist.js
file. I'm not sure for what that file exactly was used to, but it is not part anymore of the git repository. Please fix and explain why this is really not necessary anymore (I don't get it).
The rest looks good. Thx for the fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixed (Favicon related) errors which returned by Zimcheck
Fixed (Favicon related) errors which returned by Zimcheck
Fix favicons links
Remove broken links
Fix the "redundant data" error after Zimcheck
Fixes #141